Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Use more accurate test coverage. #134

Merged
merged 8 commits into from
Jan 27, 2022

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Jan 21, 2022

Use more accurate test coverage which makes our tests 63.5% covered as it now also accounts for integration tests.

@shahzadlone shahzadlone added the area/testing Related to any test or testing suite label Jan 21, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.2 milestone Jan 21, 2022
@shahzadlone shahzadlone self-assigned this Jan 21, 2022
@shahzadlone shahzadlone linked an issue Jan 21, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Jan 21, 2022

Codecov Report

Merging #134 (f75285d) into develop (3cacc40) will increase coverage by 11.00%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop     #134       +/-   ##
============================================
+ Coverage    52.86%   63.86%   +11.00%     
============================================
  Files           35       80       +45     
  Lines         3757     7328     +3571     
============================================
+ Hits          1986     4680     +2694     
- Misses        1541     2163      +622     
- Partials       230      485      +255     
Impacted Files Coverage Δ
db/tests/query/one_to_one/utils.go 100.00% <0.00%> (ø)
db/tests/query/one_to_many_multiple/utils.go 100.00% <0.00%> (ø)
query/graphql/planner/multi.go 58.06% <0.00%> (ø)
db/fetcher/fetcher.go 65.31% <0.00%> (ø)
db/tests/query/inline_array/utils.go 100.00% <0.00%> (ø)
query/graphql/planner/commit.go 83.78% <0.00%> (ø)
datastores/badger/v3/datastore.go 33.55% <0.00%> (ø)
db/base/descriptions.go 94.44% <0.00%> (ø)
db/tests/query/one_to_many/utils.go 100.00% <0.00%> (ø)
query/graphql/planner/executor.go 62.50% <0.00%> (ø)
... and 58 more

Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left a comment about the -race command based on our conversation earlier on discord. Let me know what the final descision was.

# This only covers how much of the package is tested by itself (unit test).
.PHONY: test\:coverage-quick
test\:coverage-quick:
go test ./... -race -coverprofile=coverage-quick.txt -covermode=atomic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you guys make a descision regarding the -race flag on the coverage-quick make command?

Copy link
Member Author

@shahzadlone shahzadlone Jan 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this coverage-quick rule isn't ran on any CI steps, just there for local coverage report generation of only unit tests (so no integration / cross package coverage).

We did decide to go with -race flag for the benefits. I checked locally the running time difference is like just a few seconds.

I can include the -race flag in the PR and then merge.

@shahzadlone shahzadlone merged commit 303df74 into develop Jan 27, 2022
@shahzadlone shahzadlone deleted the lone/ci/more-accurate-test-coverage branch January 27, 2022 18:42
@orpheuslummis orpheuslummis mentioned this pull request Jan 29, 2022
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
In addition to better coverage also add the `-race` flag to our checks to detect races.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Related to any test or testing suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use more accurate test coverage to cover integration tests.
2 participants